Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance redirect routes behavior #158

Merged
merged 2 commits into from
Jan 21, 2025

Conversation

maxmezzomo
Copy link
Contributor

@maxmezzomo maxmezzomo commented Nov 18, 2024

Closes #154

What I did

Replace app redirects to replace the location.

SKU and SKU Lists apps. Maybe there's more but those are the ones I found.

This prevents infinite redirects if trying to navigate back in history.

How to test

Start the server and click SKUs App, navigation should work with appropriate redirect, try to navigate back in history, you should be back to home. Can repeat for SKU Lists App.

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests).
  • Make sure to add/update documentation regarding your changes.
  • You are NOT deprecating/removing a feature.

Copy link

netlify bot commented Nov 18, 2024

Deploy Preview for commercelayer-dashboard-apps ready!

Name Link
🔨 Latest commit 4a345f2
🔍 Latest deploy log https://app.netlify.com/sites/commercelayer-dashboard-apps/deploys/678f721ab514e400082c2d8c
😎 Deploy Preview https://deploy-preview-158--commercelayer-dashboard-apps.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@maxmezzomo
Copy link
Contributor Author

if conceptually you are happy with the changes of this PR, I can commit to reset package lock to upstream. Don't know if you have conventions to always update lock but if not would say don't need for this kind of change.

@pfferrari
Copy link
Contributor

Hi Max, good catch! Thanks for your PR, we'll discuss about these changes in the next days and we'll ping you back soon.

@maxmezzomo
Copy link
Contributor Author

at your leisure 😆

@pfferrari pfferrari changed the base branch from main to enhance-redirects January 21, 2025 08:54
@pfferrari pfferrari changed the title feat: app redirects replace location Enhance redirect routes behavior Jan 21, 2025
@pfferrari pfferrari added the bug Something isn't working label Jan 21, 2025
@pfferrari pfferrari merged commit a17ce96 into commercelayer:enhance-redirects Jan 21, 2025
4 checks passed
@pfferrari
Copy link
Contributor

@maxmezzomo we added your changes to another PR #196 that is based on the updated main and almost ready to be merged.
I hope that you'll rebase everything from the actual main then to start from a clean version of the codebase/git history.
Once the new PR will be merge the issue will be closed. Thanks for your suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SKUs and SKU Lists break navigation
2 participants